chore: add ability to remove tools #1322
Conversation
| @@ -68,6 +68,13 @@ def add_tool( | |||
| self._tools[tool.name] = tool | |||
| return tool | |||
|
|
|||
| def remove_tool(self, name: str) -> None: | |||
| """Remove a tool by name.""" | |||
There was a problem hiding this comment.
We should probably say a word about the beahviour if the tool doesn't exists.
There was a problem hiding this comment.
I think this should raise if not found.
It seems something is missing, since the user doesn't have access to the underlying How is the user supposed to remove a tool? |
I was under the impression that one could obtain it from doing It feels natural to me that this code should exist, as |
|
That's a significant objection. |
Then, as a maintainer, do you have thoughts on how you would rather enable this behavior? I am not as familiar with your project. It seems reasonably clear to me that a user may want to alter the tools which are available to the MCP, in a way that is not solely static. I don't see why the tool manager must be private, nor why we could not surface some of these methods through the API. |
|
I think the current implementation (besides my comment about the exceptions) makes sense. It's just incomplete. Is the idea here to disable the tool for the whole app lifecycle or for the session? Does it even makes sense to have it disable for the app lifecycle, since the app may be running in multiple machines? I guess we want to expose a method you can call from the Context object anyway. |
|
I don't think ToolManager should be exposed. |
In my use case, it would actually be most ideal if a tool could be enabled or disabled for specific sessions (meaning, differing tools for differing consumers of a single server). I wanted to downscope this, as this is certainly more complicated than I feel I am seeking to implement right now. My understanding is that currently In my use case, I want users who are hosting their own servers (instances of the application) to be able to decide if they want to allow certain tools to be available to the consumers of their server. The best way to do this currently is with conditional declaration, which I view to be quite heavy handed. If we do not expose |
|
Oh, I now see that python-sdk/src/mcp/server/fastmcp/server.py Line 121 in 47d35f0 |
Motivation and Context
I am interested in being able to dynamically have a server offer only certain tools based on information received during its initialization. I could conditionally declare the tool, for instance:
but I perceive this to be relatively gross. I would prefer to be able to remove a tool within the API.
How Has This Been Tested?
I can achieve comparable behavior by inserting an
ifbefore defining a function with themcp.tool()decorator.Breaking Changes
No.
Types of changes
Checklist
Additional context